fix: reject NaN in numeric comparison keywords (minimum, maximum, exclusiveMinimum, exclusiveMaximum)#1506
fix: reject NaN in numeric comparison keywords (minimum, maximum, exclusiveMinimum, exclusiveMaximum)#1506gaoflow wants to merge 2 commits into
Conversation
…lusiveMinimum, exclusiveMaximum)
NaN (float('nan')) previously passed all numeric comparison checks
because Python's NaN comparisons always return False:
instance < minimum -> False (no error, should fail)
instance > maximum -> False (no error, should fail)
instance <= minimum -> False (no error, should fail)
instance >= maximum -> False (no error, should fail)
This allowed NaN-infested schemas to pass check_schema (e.g.
multipleOf=NaN, since the exclusiveMinimum:0 in the meta-schema
did not reject NaN). validate() then crashed with ValueError
inside the multipleOf handler when it tried int(quotient) on NaN.
Invert the comparisons to use the positive form:
not (instance > minimum) for exclusiveMinimum
not (instance < maximum) for exclusiveMaximum
not (instance >= minimum) for minimum
not (instance <= maximum) for maximum
For non-NaN numbers the result is identical. For NaN, all four
positive forms return False, so 'not' yields True (correctly
rejecting NaN).
Also fix the legacy draft3/draft4 equivalents in _legacy_keywords.py.
for more information, see https://pre-commit.ci
|
JSON Schema is defined on top of JSON, and JSON doesn't have NaN, so neither answer is right or wrong here really as it relates to JSON Schema behavior. Do you have anything in particular which you think motivates this change? |
|
The concrete case: NaN does reach validators in Python — stdlib |
|
My issue with this kind of thing is typically that these kinds of incremental changes outside the spec still leave other behaviors unfixed, and worse, they communicate to people that outside of the spec is in some way "supported", in a way which means that future potentially harder-to-implement changes (including ones we haven't thought of) end up on the table. For example, what do we do about: The alternative (the "status-quo") is "it's undefined behavior anytime you have non-JSON values in your Python objects". |
2 participants
NaN (
float("nan")) currently passes all numeric comparison checks because Python's NaN comparisons always returnFalse. This has two consequences:Schema validation bypass:
check_schemadoes not reject schemas containing NaN in numeric fields. For example,{"multipleOf": float("nan")}passes schema validation because the meta-schema'sexclusiveMinimum: 0fails to reject NaN (NaN <= 0isFalse). This then causesvalidate()to crash withValueError: cannot convert float NaN to integerinside themultipleOfhandler.Instance validation bypass: Instances containing NaN pass any
minimum/maximum/exclusiveMinimum/exclusiveMaximumconstraint, even though NaN is not a valid JSON number and cannot satisfy any ordering relationship.The fix inverts the comparison logic to use the positive (non-negated) form:
not (instance > minimum)instead ofinstance <= minimumnot (instance < maximum)instead ofinstance >= maximumnot (instance >= minimum)instead ofinstance < minimumnot (instance <= maximum)instead ofinstance > maximumFor normal numbers the result is identical. For NaN, all four positive comparisons return
False, so the negation correctly yieldsTrue(rejecting NaN). The same fix is applied to the legacy draft3/draft4 equivalents in_legacy_keywords.py.